feat: socket #445
feat: socket #445
Conversation
thantos
force-pushed
the
sussman/feat/socket
branch
from
172358e
to
050e656
Compare
| this.socketHandlersPrincipal = | ||
| this.socketHandlersList.length > 0 || this.local | ||
| ? new DeepCompositePrincipal( | ||
| ...(this.local ? [this.local.environmentRole] : []), | ||
| ...this.socketHandlersList.map((f) => f.grantPrincipal) | ||
| ) | ||
| : new UnknownPrincipal({ resource: this }); |
There was a problem hiding this comment.
nit: generalize this redundant stuff in a function
| $connect: ( | ||
| request: { | ||
| connectionId: string; | ||
| query?: Record<string, string | undefined>; | ||
| }, | ||
| context: SocketHandlerContext | ||
| ) => Promise<void> | void; | ||
| $disconnect: ( | ||
| request: { connectionId: string }, | ||
| context: SocketHandlerContext | ||
| ) => Promise<void> | void; | ||
| $default: ( | ||
| request: { | ||
| connectionId: string; | ||
| body?: string; | ||
| headers?: Record<string, string | undefined>; | ||
| }, | ||
| context: SocketHandlerContext |
There was a problem hiding this comment.
Should we drop the $ prefix?
There was a problem hiding this comment.
the benefit of keeping them is that we could support custom route names in the same object.
{
$connect: () => {},
$disconnect: () => {},
$default: () => {},
default: () => {} // custom route key named default
}
| defaultRouteOptions: { | ||
| // https://stackoverflow.com/a/72716478 | ||
| // must create one integration per... | ||
| integration: new WebSocketLambdaIntegration("default", this.handler), | ||
| }, | ||
| connectRouteOptions: { | ||
| integration: new WebSocketLambdaIntegration("Connect", this.handler), | ||
| authorizer: new WebSocketNoneAuthorizer(), | ||
| }, | ||
| disconnectRouteOptions: { | ||
| integration: new WebSocketLambdaIntegration("Disconnect", this.handler), |
There was a problem hiding this comment.
Dang, those LambdaIntegrations. I wonder if we can work around that?
Not high priority, but gah-damn that is bad
| public execute(call: SocketCall): Promise<CallOutput<SocketCall>> { | ||
| return this.socketClient[call.operation.operation]( | ||
| call.operation.socketName, | ||
| // @ts-ignore - typescript won't let me case the params... |
| withMiddlewares<CommandContext, HttpResponse, HttpRequest>( | ||
| command.middlewares ?? [], |
There was a problem hiding this comment.
Generalized the function to work for both socket and command.
| SocketResponse, | ||
| SocketConnectionRequest | ||
| >( | ||
| socket.connectMiddlewares, |
There was a problem hiding this comment.
Middleware is only on connect?
There was a problem hiding this comment.
Yeah. Web sockets are asymmetrical. You only get query strings during connect. Apig only allows authorizers for the connect route.
Though now that I think about it, it may be useful to pass all of the request types through with the route key to give the option to impact other routes. I was just thinking about auth.
Need to look at how apig handles the identity too.
Same guarantee as the other side effects (emit event and send signal), which is there is none and currently no reporting of the errors. I think once we move the calls to be downstream of the dynamo write, we can add a DLQ, reporting, and retry logic to the side effects. |
Tasks:
Socket Handlerdisplay/events.tswith new events/fields